Conversation
src/Property/Selector.php
Outdated
| public static function isValid($sSelector) | ||
| { | ||
| return preg_match(static::SELECTOR_VALIDATION_RX, $sSelector); | ||
| return preg_match(static::SELECTOR_VALIDATION_RX, $sSelector) && strpos($sSelector, ': ') === false; |
There was a problem hiding this comment.
I guess this check is probably too simplistic since it will fail some valid selectors, e.g.:
a[href*=": "]#some_id_that_ends_in_a\: > a
There was a problem hiding this comment.
Thanks! These should be fixed now :)
8d364f3 to
d43033b
Compare
The invalid selectors shouldn't be included in the parsed tree I think. Take a look at this fiddle comparing how the browser parses the invalid cases: https://jsfiddle.net/sm67z4nb/ They do not seem to be included in the final tree. I like the idea of the debugging tools btw :) |
You're right that if there's something in any of a possibly comma-separated list of selectors the browser doesn't recognize, the entire rule is dropped. For example, before It was not possible to put the selectors in a comma-separated list, because each browser would barf upon encountering a rival's vendor prefix. Obviously the parser shouldn't be so prejudiced. But if there is a selector consutruct which is invalid for all browsers, it will never work, and perhaps should be dropped. Either way, such situation would be an opportune use for the logger (TBI). @sabberworm, @oliverklee, WDYT - should we drop rules with selector constructs that appear to be invalid? We would need to be careful not to drop actually-valid constructs. |
|
Yes, let's drop them. To quote from https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_syntax/Error_handling:
|
Take care not to confuse syntactically-invalid rules with rules that are syntactically valid but a specific browser doesn’t know the semantics of. However, to complicate matters, we also have some syntactically-invalid stuff that we do parse because they were wildly-deployed workarounds of browser bugs. IMHO, we should validate selectors only once we actually parse them. |
Pseudo classes cannot have a space after the
:sign. We should consider such occurrences invalid.